-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
@mxnet-label-bot add [pr-awaiting-review] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@aaronmarkham for review/merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not going to hold this up with my testing as my hardware is Pi Model B v2 and only has 512MB of RAM. I keep getting stuck trying into install numpy. The changes look fine, but at the moment I can't validate the functionality of the tutorial.
One thing to note though - the instructions tell you to follow the install page's setup, but those tell you to create a Python 3 env, whereas this tutorial is 2.7. Minor detail, but maybe this could be fixed.
Here's the flow I started (but stopped at the install step as it is hanging):
Format your SD card with SD Card Formatter.
Download, unzip, then copy contents of the NOOBS (network install) zip file to SD card.
Boot up the Pi and follow the prompts to install the headless Raspbian (smaller and might help with memory issues).
Login to the pi with user pi
and password raspberry
.
Install pip with:
sudo apt-get install python-pip
Install virtualenv with:
sudo pip install virtualenv
Create a Python 2.7 environment for MXNet with:
virtualenv -p `which python` mxnet_py27
Download the mxnet wheel with:
wget https://mxnet-public.s3.amazonaws.com/install/raspbian/mxnet-1.5.0-py2.py3-none-any.whl
Install mxnet with:
pip install mxnet-1.5.0-py2.py3-none-any.whl
That wheel is something I created. It would be nice if we hosted some tagged versions somewhere, so people can grab a wheel with v1.3.1 or the latest RC.
Also, I don't see that this tutorial has a link from the tutorial's index page. So how does one find this tutorial?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still haven't seen this work yet, but I can tell you that the earlier step where you copy the model params file is incorrect. The file listed isn't in the tar. Also the dependencies setup isn't complete.
I have my notes on how to get MXNet running on a Pi 3 with Stretch here:
https://gist.github.com/aaronmarkham/b3d0c715abca92ccc4b79ed0f19d1916
A Pi 2 doesn't work with MXNet (using the cross-compiled wheel) - it core dumps.
One further issue with this tutorial is that it requires OpenCV2 to be installed separately. This would require a build from source as there doesn't seem to be a package available for Stretch. Then I ran out of disk space on my 8GB card before I could go further. This could be avoided though since the image processing steps are already part of MXNet's port of OpenCV operators. We don't need OpenCV 2 here.
I'm not going to approve or request changes. If you know this update works on your test devices that say so, and we'll merge this update. However, I think that this tutorial needs to be modernized to use the latest MXNet features for handling images. And we should also have the Install page updated so that it has the appropriate dependencies.
Additionally, I found it a pain to have to compile on Ubuntu, and I really wish that we offered wheels prepped for whatever Pi & MXNet versions we can support.
I put the edits for MXNet installation on Raspberry Pi in this PR: #14172 |
I also just fixed the conflict from the new license header. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@larroy Could you look into comments by @aaronmarkham |
@@ -100,13 +101,14 @@ with open('synset.txt', 'r') as f: | |||
synsets = [l.rstrip() for l in f] | |||
|
|||
# Load the network parameters | |||
sym, arg_params, aux_params = mx.model.load_checkpoint('Inception_BN', 0) | |||
sym, arg_params, aux_params = mx.model.load_checkpoint('Inception-BN', 126) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
magic number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the number of epoch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, what I mean is this probably shouldn't be hard-coded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, but if you follow the instructions, the model donloaded has that epoch for the checkpoint. What do you suggest instead?
@mxnet-label-bot update [Example, pr-awaiting-response] |
@larroy Could you please address the review comments? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the fix! LGTM
@larroy Could you please give an update on the PR? Thanks |
@larroy Gentle ping... |
@aaronmarkham @larroy Is this PR good to go? |
@larroy has an unresolved comment here. Once that's done and he's sure it works, I'm ok with the PR. |
@aaronmarkham I responded to the comment. |
@mxnet-label-bot remove [pr-awaiting-response] |
@mxnet-label-bot add [pr-awaiting-review] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you!
Description
see tittle
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.